Pass NULL walkState to jvmti callback for JNI local reference on stack #18394
Conversation
We could not pass walkState parameter to jvmti callback for JNI local reference on stack (except normal Stack slot), it could cause jvmti crash, so pass walkState == NULL for the case. Signed-off-by: hulin <[email protected]>
|
Not expert on this, but at least I can see it's consistent with how we call it when we find it as a JNI thread slot (not a stack one). |
|
jenkins compile win jdk21 |
|
is there a value in specializing code so that the type is set to ROOT or SLOT respective to if referrer is NULL or non-null? |
|
In other words, the fact that we will now pass NULL as referrer will mislead a callback believing this is a thread root slot, rather then a stack one. Could that a problem for any callback that you can think of? Perhaps, the answer is that this reference being on a stack is just a side effect of an optimization that a first few are put on the stack rather than on thread local list, but effectively regardless where they are stored they should be treated as if they were stored into the list? |
| doSlot(slotPtr, J9GC_ROOT_TYPE_JNI_LOCAL, -1, NULL); | ||
| } else { | ||
| doSlot(slotPtr, J9GC_ROOT_TYPE_STACK_SLOT, -1, (J9Object *)walkState); |
There was a problem hiding this comment.
I thought the cast was suspicious when I saw #18379, NULL seems more reasonable, but why should the call on line 651 not also pass NULL?
There was a problem hiding this comment.
A valid question. In theory, callbacks being aware that the type is STACK_SLOT could safely cast the referrer parameter back to walkState type and do something about that info/structure (print stack slot address etc), but I don't know if they do.
I'm ok with passing NULL, but don't have the whole picture (history and awareness of current callbacks) to say it's really safe.
There was a problem hiding this comment.
If we pass NULL for STACK_SLOT, it will break the expectation in jvmtiHeap10.c::processStackRoot() function.
After reviewing the VM code which uses the only use case that I see in the VM is in This callback is used by Given that there is no testing that reflect this problem, I will open a new issue to highlight failure case in the current implementation and work on a PR to correct this. |
|
If we continue to pass |
We could not pass walkState parameter to jvmti callback for JNI local reference on stack (except normal Stack slot), it could cause jvmti crash, so pass walkState == NULL for the case.
fix: #18392
Signed-off-by: hulin [email protected]